-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat (INVT-CPC 885 ) : Ability to get product by id and details page #541
Feat (INVT-CPC 885 ) : Ability to get product by id and details page #541
Conversation
…s and the inventory id also, and theres a back button that goes to products
, 'visits', 'vetList','vetForm','vetDetails', 'visitList', 'billForm', 'billUpdateForm', 'loginForm', 'rolesDetails', 'signupForm', | ||
'billDetails', 'billsByOwnerId', 'billHistory','billsByVetId','inventoryList', 'inventoryForm', 'productForm','inventoryProductList', 'inventoryUpdateForm', 'productUpdateForm' | ||
, 'verification' , 'adminPanel','resetPwdForm','forgotPwdForm','petTypeList']); | ||
, 'visits', 'vetList','vetForm','vetDetails', 'visitList', 'billForm', 'billUpdateForm', 'loginForm', 'rolesDetails', 'signupForm', 'productDetailsInfo', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you think it was necessary to add "productDetailsInfo" in the app.js?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If i didn't add the product detail info, in the app.js, then my page would never render when called.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's very smart thank you for your input Mr Siddiqui.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approve of the code everything looks good. Look at my comments that I left about the big-warning on the edit button because I coded that part and wanted to know your thought process on changing it. Other then that great work Abdur keep it up, brother.
, 'visits', 'vetList','vetForm','vetDetails', 'visitList', 'billForm', 'billUpdateForm', 'loginForm', 'rolesDetails', 'signupForm', | ||
'billDetails', 'billsByOwnerId', 'billHistory','billsByVetId','inventoryList', 'inventoryForm', 'productForm','inventoryProductList', 'inventoryUpdateForm', 'productUpdateForm' | ||
, 'verification' , 'adminPanel','resetPwdForm','forgotPwdForm','petTypeList']); | ||
, 'visits', 'vetList','vetForm','vetDetails', 'visitList', 'billForm', 'billUpdateForm', 'loginForm', 'rolesDetails', 'signupForm', 'productDetailsInfo', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad that you found out, that the order matters when setting the module on the app.js when it comes to loading new files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, took me a while to figure it out but after doing some debugging found my issue.
<td><a ui-sref="updateProductInventory({inventoryId: $ctrl.inventoryProductList[0].inventoryId, productId: product.productId})"> | ||
<button class="add-bundle-button btn btn-success">Edit</button> | ||
<button class="add-bundle-button btn btn-warning">Edit</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate on why you changed the btn-success to the btn-warning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My pleasure, so I changed the color from btn -success to btn - warning, because green means success and you haven't changed or modified any value yet, thats why i changed the btn color to yellow to show that you need to test the functionality of the button.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see thank you for informing me. It does make sense now because it will also match better with the inventory since the other update button in the inventories has it on "Warning" as well now that I look at it.
Qodana for JVM341 new problems were found
☁️ View the detailed Qodana report Contact Qodana teamContact us at [email protected]
|
JIRA: link to jira ticket
https://champlainsaintlambert.atlassian.net/browse/CPC-885
Context:
Changes
Before and After UI (Required for UI-impacting PRs)
Before
After
When you press the back products button it brings you back to inventory products page